Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faster matrix sqrt for upper triangular matrices. #31100

Merged
merged 1 commit into from
May 2, 2020

Conversation

mateuszbaran
Copy link
Contributor

Replacing Val(::Bool) in an argument list with an explicit if makes the
call type stable. The issue was discussed in #31007.

Copy link
Member

@andreasnoack andreasnoack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Would be great if you could also prepare a benchmark for this in BaseBenchmarks. It's not required for this PR but it would be great to have to avoid that this regresses.

@andreasnoack andreasnoack added performance Must go faster linear algebra Linear algebra labels Feb 18, 2019
@mateuszbaran
Copy link
Contributor Author

No problem, I'll make a PR in BaseBenchmarks for this issue.

mateuszbaran added a commit to mateuszbaran/BaseBenchmarks.jl that referenced this pull request Feb 18, 2019
mateuszbaran added a commit to mateuszbaran/BaseBenchmarks.jl that referenced this pull request Feb 18, 2019
@dkarrasch
Copy link
Member

@mateuszbaran Could you please rebase this PR, and when tests pass, I'll merge. Or has this been fixed by type inference improvements by now?

@mateuszbaran
Copy link
Contributor Author

I've actually forgotten about this PR. I will check if this is still relevant.

Replacing Val(::Bool) in an argument list with an explicit if makes the 
call type stable. The issue was discussed in JuliaLang#31007.
@mateuszbaran
Copy link
Contributor Author

OK, this still looks relevant, there is a similar difference in performance on 1.4.1 and a 44 day old Julia master I had around. I think my branch is rebased now.

@dkarrasch
Copy link
Member

Sorry to bother you again, but can you rebase again. It's not your fault, but you rebased onto a commit that had a whitespace issue.

@dkarrasch
Copy link
Member

Nevermind, I'll just merge. The whitespace issue is resolved on master, and this PR doesn't touch the file in question.

@dkarrasch dkarrasch merged commit 68c8708 into JuliaLang:master May 2, 2020
@mateuszbaran
Copy link
Contributor Author

OK, thanks 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants